-
-
Notifications
You must be signed in to change notification settings - Fork 7.4k
[R] fix set[object] deserialization #22697
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[R] fix set[object] deserialization #22697
Conversation
| if (is.list(obj) && length(obj) == 1 && is.data.frame(obj[[1]])) { | ||
| obj <- obj[[1]] | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This situation arises when parsing responses that have fields that are arrays of other response models. E.g. consider a Person response with a nested family field populated with an array of Relationship objects
txt <- '[{"name": "bob", "family":[{"name":"joe", "relationship":"dad"},{"name":"sue", "relationship":"mom"}]}]'
json <- jsonlite::fromJSON(txt)
class(json$family)
#> [1] "list"
class(json$family[[1]])
#> [1] "data.frame"Here the family field is initially parsed as a list of length 1, populated with a data.frame. This is not handled in the current code because nrow(json$family) is NULL. This is why we get some unexpected data.frame return values where we should get a list of R6 objects.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 3 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="modules/openapi-generator/src/main/resources/r/api_client.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/r/api_client.mustache:379">
P2: Nested collections with single outer element are flattened: unwrapping list-of-data.frame drops one collection dimension</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
2 issues found across 6 files (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="samples/client/petstore/R-httr2-wrapper/R/api_client.R">
<violation number="1" location="samples/client/petstore/R-httr2-wrapper/R/api_client.R:380">
P2: Non-primitive collections that jsonlite parses as lists (no `nrow`) return NULL instead of deserializing elements, dropping set/array of objects.</violation>
</file>
<file name="modules/openapi-generator/src/main/resources/r/libraries/httr2/api_client.mustache">
<violation number="1" location="modules/openapi-generator/src/main/resources/r/libraries/httr2/api_client.mustache:391">
P2: Nested collections are flattened: unwrapping `list(data.frame)` collapses the outer collection when deserializing `collection[collection[T]]`, returning the wrong shape for length-1 outer arrays.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
modules/openapi-generator/src/main/resources/r/libraries/httr2/api_client.mustache
Show resolved
Hide resolved
|
r integration tests passed via #22721 |
|
thanks! |
@Ramanth, @saigiridhar21, @wing328
This PR handles 2 related issues pertaining to deserializing responses in generated R clients.
deserializeObj()method of the API client only supportedarray[...]types, notset[...]types. That is fixed here by collapsing both to a temporarycollection[...]type which is only used for a regex to identify collections and pull out the inner type.jsonlitelibrary will sometimes parse nested JSON arrays as an Rdata.framenested within an Rlist. I've provided an example as a comment near the change. This PR handles this case so that nested objects are correctly parsed into the target R6 objects. Current behavior is that they are returned, unexpectedly, as data frames.PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master(upcoming7.x.0minor release - breaking changes with fallbacks),8.0.x(breaking changes without fallbacks)"fixes #123"present in the PR description)Summary by cubic
Fixes deserialization in generated R clients. Adds support for set[...] and correctly parses nested arrays from jsonlite so APIs return proper R6 objects instead of data.frames.
Written for commit 511f575. Summary will update on new commits.